Feature/remove redialing #842
Conversation
| @@ -66,16 +56,8 @@ type ManagedTransport struct { | |||
| LogEntry *LogEntry | |||
| logUpdates uint32 | |||
|
|
|||
| dc DiscoveryClient | |||
| ls LogStore | |||
| ebc *appevent.Broadcaster | |||
There was a problem hiding this comment.
We still need this structure though? Adding new transports at runtime is required to trigger events to be broadcasted to VPN for example. Am I missing somehting?
There was a problem hiding this comment.
I checked and they were not used anywhere else except for some tcp events. I moved those events to transport manager itself to avoid passing those around, but perhaps they should be kept here.
|
Please also remove the |
Previously on transport save the whole manager would be blocked until the whole process is finished Now, saveTransport method only locks in critical sections, and sometimes uses lighter locks (RLock). This allows multiple saveTransport methods to run simultaneously
|
@ersonp as @i-hate-nicknames is out, I would like you to take over the rest of the comments as they are not too broad in scope or deep in effect. After changes are made, make sure to give a good bit of testing to the overall PR and we should get this merged. |
There was a problem hiding this comment.
API looks good! Now that I see it, I am not sure if Senyoret actually needs the GetPersistentTransports or whether it would be more convenient to get it in the Summary call. @Senyoret1 let us know.
|
If persistent and non-persistent transports are going to be shown just as the transports are being shown at this moment in the UI, having all the data in the summary would be the best option, as no changes in the UI would be needed. If you want the UI to do something special with persistent transports, then I would need more info to know what would have to be done and what will be nedded |
|
We will need to show the list of persistent transports set up by the user as well as a way for the user to add or remove them. Is there anything more that needs to be done @jdknives |
Did you run
make format && make check?Fixes #830
Fixes #844
Fixes #843
Changes:
How to test this PR:
A known issue is that sudph transports do not get closed automatically, when other end of connection closes. This could be an issue with kcp-go library configuration, or it could be the case that the library doesn't support it.
For this case, I added an automatic connection close logic, built on
net.Conndeadline logic. Basically, the idea is to update deadline every time the connection is used.For this to work properly, we need to add support for some kind of keep-alive messages to the transport layer.
Currently, it is disabled (deadline is 0 and ignored by conn).
Shutdown logic is also fixed in this PR, since I thought it would help with sudph issues.